-
Couldn't load subscription status.
- Fork 118
Add accessor for Binary data
#1383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1383 +/- ##
==========================================
+ Coverage 84.65% 84.68% +0.02%
==========================================
Files 115 115
Lines 29557 29641 +84
Branches 29557 29641 +84
==========================================
+ Hits 25021 25100 +79
- Misses 3329 3330 +1
- Partials 1207 1211 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. If we do this, we need to implement this for ArrowEngineData don't we?
Also, so far the visitors have been for extracting log data, and not any generic data. What's the use-case for this visitor?
That's a good call. It does look like we need to use |
| let mut visitor = BinaryVisitor { values: vec![] }; | ||
| arrow_data.visit_rows(&[ColumnName::new(["data"])], &mut visitor)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also do a test where we apply BinaryVisitor on something like ints or long?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems reasonable 👍
What changes are proposed in this pull request?
Adds an accessor for Binary blobs
Resolves #1382
How was this change tested?